Fix BookListSerializer update example logic to support creation#9990
Fix BookListSerializer update example logic to support creation#9990JPisOP007 wants to merge 1 commit into
Conversation
|
Logic looks correct overall — nice catch on the Avoid iterating This keeps the same result but only goes through Is there a docs test covering this example? DRF sometimes has tests that verify code examples in the docs actually run correctly. If one exists for this section, it might need updating too — worth checking. Otherwise this is a solid fix for a real bug in the documentation. Thanks for working on it! |
ulugbekbackend
left a comment
ulugbekbackend
left a comment
There was a problem hiding this comment.
Overall logic is solid — good fix for the KeyError on creation without an id. Left a couple of inline notes.
| book_mapping = {book.id: book for book in instance} | ||
| data_mapping = {item['id']: item for item in validated_data} | ||
| data_mapping = {item['id']: item for item in validated_data if 'id' in item} | ||
| new_items = [item for item in validated_data if 'id' not in item] |
There was a problem hiding this comment.
Nit: this could be a single loop instead of two separate comprehensions, to avoid iterating validated_data twice — e.g.:
data_mapping = {}
new_items = []
for item in validated_data:
if 'id' in item:
data_mapping[item['id']] = item
else:
new_items.append(item)
| # so use a writable field here, rather than the default which would be read-only. | ||
| id = serializers.IntegerField() | ||
| # The id is optional so that new items (without an id) can be created. | ||
| id = serializers.IntegerField(required=False) |
There was a problem hiding this comment.
Makes sense given the new create path.
2 participants
Description
This PR fixes a bug in the documentation example for
BookListSerializer.update()under theListSerializersection of the serializers API guide.Problem
The current documentation example crashes with a
KeyErrorif list items are submitted without anidfield. This occurs when trying to perform creations alongside updates via aListSerializer.Solution
data_mappingto only include validated data items containing an'id'key (updates/deletes).new_itemsthat do not contain an'id'key (creations).new_itemsand called.create(data)on the child serializer (self.child.create(data)).BookSerializerdefinition in the example to useid = serializers.IntegerField(required=False). This ensures validation succeeds when creating new items that do not yet have an ID.refs #9469